Found 2 medium-risk issues in the new model-selection support:

1. parseDAG() / validateModelMap() now canonicalize string model overrides into { id } objects, which changes the observable return shape of the exported parsing helpers for string-only inputs.
2. Param validation assumes Cursor.models.list() always populates parameters, but the SDK marks that field optional, so models that expose only variants would reject otherwise valid param selections.

  

_{Sent by Cursor Automation: Flatbread PR Review}

Code Review — feat: support Cursor SDK model params in proof

This is a well-structured feature. The separation of concerns across validation, normalization, catalog-backed resolution, and variant scoring is clean, defensive cloning throughout prevents aliasing bugs, and the eager validation loop at startup (for (const complexity of COMPLEXITY_KEYS)) fails fast before any agent work begins. The --init-only bypass for the SDK call is correctly documented. Good work overall.

Actionable feedback below (no blockers):

  

_{Sent by Cursor Automation: Flatbread PR Review}

Review verdict

REQUEST_CHANGES — one implementation/doc contradiction that will silently break existing --models-file files, one unsupported breaking public-API removal, no error handling on the new startup network call, and zero test coverage for 222+ lines of new branching logic that is correctness-critical.

---

Chunk-bound feedback

packages/proof/src/dag.ts:511–514

• severity: HIGH
• flagged-by: correctness-and-contracts, proof-runtime-internals
• risk: validateModelMap calls validateModelSelection, which calls validateModelSelectionObject and rejects any non-object input with "must be a model object." A user's existing --models-file or inline DAG models that uses the old plain-string format ("HIGH": "claude-opus-4-7") will throw at parse time with a cryptic error, not a migration notice. normalizeModelSelection was introduced precisely to accept string | ModelSelection — it is the right function for the parse path.
• minimal fix: Replace the validateModelSelection(value, ...) call in validateModelMap with normalizeModelSelection(value as ModelSpec, ...) so the function accepts both the legacy string format and the new object format.

packages/proof/src/index.ts:14

• severity: HIGH
• flagged-by: correctness-and-contracts
• risk: createModelResolver is removed from the public exports of @flatbread/proof with no deprecation shim. Any external consumer importing it (including the orchestrator DAG files and any user-authored tooling) gets a TypeScript compile error with no actionable message. The return type also changed from (c) => string to (c) => ModelSelection, which is an additional silent breakage for consumers who stored the inferred return type.
• minimal fix: Re-export createModelResolver as a deprecated alias: export const createModelResolver = createModelSelectionResolver; with a @deprecated JSDoc, and bump the minor version with a migration note.

packages/proof/src/run_dag.ts:528–543

• severity: MED
• flagged-by: proof-runtime-internals
• risk: await Cursor.models.list() is called at the top of main() with no try/catch. A transient network error, a rate-limit response, or a missing CURSOR_API_KEY at this point kills the entire proof run before any task has executed. For long DAGs (e.g. the PMF audit) this means paying the startup cost with nothing to show for it. The existing CURSOR_API_KEY check two lines above fires before we even reach this code, so the failure mode here is specifically network/rate-limit transience.
• minimal fix: Wrap the catalog fetch in a try/catch; on failure, warn to stderr ([proof] WARNING: catalog validation skipped — Cursor.models.list() failed: <err.message>) and fall back to unresolvedModelForComplexity so the DAG can still launch with unvalidated model ids.

.cursor/skills/proof/SKILL.md:176

• severity: MED
• flagged-by: correctness-and-contracts, proof-runtime-internals
• risk: Line 176 states "Values can be plain SDK model id strings or SDK model selections with params", but line 58 in the same file states "Values must be model selection objects." The implementation (validateModelMap) only accepts objects. One of these statements must be wrong — and if the intent is that strings should work (as line 176 and the README imply), then the code has the bug described above in dag.ts:511–514.
• minimal fix: Once dag.ts:511–514 is fixed to use normalizeModelSelection, update line 58 to match line 176: both formats are valid.

---

Coverage plan

1. packages/proof/src/__tests__/dag.test.ts — positive: validateModelMap accepts plain string values ({"HIGH": "claude-opus-4-7"}) after the normalizeModelSelection fix
2. packages/proof/src/__tests__/dag.test.ts — positive: validateModelMap accepts full object values ({"HIGH": {"id": "claude-opus-4-7", "params": [...]}})
3. packages/proof/src/__tests__/dag.test.ts — negative: validateModelSelection rejects null, array, missing id, empty string id
4. packages/proof/src/__tests__/dag.test.ts — negative: validateModelParams rejects duplicate param ids; rejects non-array params
5. packages/proof/src/__tests__/dag.test.ts — positive: normalizeModelSelection converts a plain string to {id: string} and passes an existing object through unchanged
6. packages/proof/src/__tests__/dag.test.ts — edge (unknown model): resolveModelSelectionFromCatalog throws with the full known-models list when the id is not in the catalog
7. packages/proof/src/__tests__/dag.test.ts — positive: resolveModelSelectionFromCatalog returns clone of selection for a model with no variants
8. packages/proof/src/__tests__/dag.test.ts — positive: resolveModelSelectionFromCatalog picks the default variant when no params are requested
9. packages/proof/src/__tests__/dag.test.ts — edge (partial params): chooseMatchingVariant scores variants by closeness-to-defaults for un-specified params and picks the best-fit; tie-breaks to the catalog-declared default
10. packages/proof/src/__tests__/dag.test.ts — negative: resolveModelSelectionFromCatalog throws when requested params match no variant
11. packages/proof/src/__tests__/dag.test.ts — positive: createCatalogBackedModelResolver returns a cloned (not the same reference) ModelSelection on repeated calls for the same complexity (cache isolation)
12. packages/proof/src/__tests__/dag.test.ts — negative: resolveModelSelectionFromCatalog throws when a model has neither parameters nor variants and params are requested

---

Suggested follow-ups

• Consider whether --models-file should support a "$schema" field pointing to a generated JSON Schema for model selections, so editors can validate the file before running proof.
• The scoreVariant heuristic (prefer defaults for un-specified params) is not documented in a comment; add an inline explanation so future maintainers understand why the scoring works the way it does.

---

Reviewer scoreboard

• correctness-and-contracts: 3 findings, 0 coverage gaps, signal: HIGH
• proof-runtime-internals: 2 findings, 0 coverage gaps, signal: HIGH
• test-coverage-robustness: 0 non-test findings, 12 coverage gaps, signal: HIGH

  

_{Sent by Cursor Automation: Flatbread PR Review}

Code Review — feat: support Cursor SDK model params in proof

Verdict: REQUEST_CHANGES — Two independent consensus HIGH findings across all three reviewer perspectives: (1) SKILL.md:176 declares string values valid for the models field while validateModelMap calls validateModelSelection which rejects non-objects, creating a published-doc / enforced-contract break; (2) Cursor.models.list() is called bare at startup with no error handling, meaning a transient network error kills the entire run before any task executes.

---

Chunk-bound feedback

packages/proof/src/dag.ts:511-514 — severity: HIGH — flagged-by: all three perspectives

validateModelMap calls validateModelSelection, which throws "must be a model object" for any plain string. SKILL.md:176 explicitly tells users "Values can be plain SDK model id strings." A user who follows the docs will receive a runtime error with no indication the doc is wrong.

Minimal fix (pick one and apply it everywhere):

• Option A — "objects only" (least code change): keep validateModelSelection but remove the "plain SDK model id strings" clause from SKILL.md:176, and update SKILL.md:58 to say "must be model selection objects" consistently.
• Option B — "strings or objects": change validateModelMap to call normalizeModelSelection(value, ...) instead, which accepts both, then fix SKILL.md:58 to match.

Either way, add a negative test: validateModelMap({ HIGH: "claude-opus-4-7", ... }) should throw (option A) or pass (option B), pinning whichever contract is chosen.

---

packages/proof/src/run_dag.ts:530-543 — severity: HIGH — flagged-by: proof-runtime-internals, test-coverage-robustness

await Cursor.models.list() is called unconditionally with no try/catch. A transient network hiccup, rate-limit response, or expired CURSOR_API_KEY causes an unhandled rejection that terminates the process before a single task runs, with no recovery path and no user-actionable message.

Minimal fix: Wrap in try/catch and throw a clear error: "Could not fetch Cursor model catalog — check CURSOR_API_KEY and network connectivity. Original error: ...".

---

packages/proof/src/canvas_writer.ts:236-246 — severity: MED — flagged-by: proof-runtime-internals

ModelParameterValue and ModelSelection are redeclared inside the canvas template with only a // Keep in sync comment as guard. TypeScript cannot inspect the embedded string template. If dag.ts adds a field to either interface, the canvas will compile fine but silently render stale data.

Minimal fix: Add a type-level assertion in canvas_writer.ts (outside the template string): type _AssertModelSelectionSync = import('./dag.js').ModelSelection extends ModelSelection ? true : never; to make divergence a compile error. Long-term: extract the template into a file that can import from dag.ts directly.

---

Coverage plan (critical gaps)

1. packages/proof/src/dag.test.ts — negative — validateModelMap({ HIGH: "claude-opus-4-7", ... }) behavior must be pinned (throw or not) based on the chosen contract.
2. packages/proof/src/dag.test.ts — positive — validateModelMap({ HIGH: { id: 'x' }, MED: { id: 'y' }, LOW: { id: 'z' } }) round-trips correctly (zero tests for the happy path).
3. packages/proof/src/dag.test.ts — negative — validateModelSelection({}) throws with label-prefixed message; validateModelSelection({ id: '' }) throws; validateModelSelection({ id: 'x', params: 'bad' }) throws.
4. packages/proof/src/dag.test.ts — positive — createModelSelectionResolver() with no overrides returns DEFAULT_MODEL_MAP[c] shape for each of HIGH/MED/LOW.
5. packages/proof/src/dag.test.ts — positive — createModelSelectionResolver({ HIGH: { id: 'x' } }) returns override for HIGH and defaults for MED/LOW.
6. packages/proof/src/dag.test.ts — positive — formatModelSelection({ id: 'x' }) → 'x'; with params → 'x (effort=max)'.
7. packages/proof/src/dag.test.ts — positive — createCatalogBackedModelResolver resolves all three complexities; propagates error on unknown model id.
8. packages/proof/src/dag.test.ts — edge:empty-params-stripping — normalizeModelSelection({ id: 'x', params: [] }) returns { id: 'x' } with no params key.
9. packages/proof/src/dag.test.ts — edge:duplicate-params — normalizeModelSelection with two params sharing the same id throws with /duplicate id/.
10. packages/proof/src/dag.test.ts — edge:cache-isolation — second call to createCatalogBackedModelResolver for the same complexity is deepEqual but not === the first call.
11. packages/proof/src/dag.test.ts — edge:legacy-resume-path — taskModelSelection with ts.modelSelection absent and ts.model: "claude-opus-4-7" produces { id: "claude-opus-4-7" }.
12. packages/proof/src/dag.test.ts — edge:both-declared — resolveModelSelectionFromCatalog for a catalog item with both parameters and variants resolves via the variant path.

---

Suggested follow-ups (out of scope)

• --models-file parsing path: Confirm it also calls validateModelMap (or equivalent) and not an older string-only parser.
• SDK params runtime handling: TypeScript structural compatibility is confirmed, but whether the SDK actually sends params to the inference endpoint is not testable from this repo. Add an integration note or smoke test once the SDK behavior is confirmed.
• Cursor.models.list() retry policy: Once the try/catch is added, evaluate whether one retry with exponential backoff is warranted for this startup-blocking call.
• canvas_writer.ts long-term: Extract the template into a separate file that can import from dag.ts directly.

---

Reviewer scoreboard

• proof-runtime-internals: 3 findings, 5 coverage gaps, signal: HIGH
• correctness-and-contracts: 3 findings, 6 coverage gaps, signal: HIGH
• test-coverage-robustness: 2 findings, 14 coverage gaps, signal: MED

  

_{Sent by Cursor Automation: Flatbread PR Review}

Review verdict

REQUEST_CHANGES — two prior findings remain unaddressed: (1) Cursor.models.list() is called without error handling, meaning any transient SDK/network failure kills the entire DAG run before a single task starts; (2) ModelParameterValue and ModelSelection are still duplicated verbatim inside the canvas template string with only a comment as the sync guard.

---

Good progress since last run

• validateModelId correctly renamed to validateNonEmptyString — removes misleading specificity.
• taskModelSelection legacy fallback now carries an explanatory comment; the ts.model assumption (always a bare id in pre-this-PR persisted state) is documented.
• Tie-breaking tests added: highest-scoring variant wins, equal-score tie breaks to catalog default. Both paths covered.
• validateModelMap now routes through normalizeModelSelection, accepting both plain string and object inputs.
• SKILL.md documentation contradiction resolved — "can be plain strings or objects" is used consistently.

---

Chunk-bound feedback

packages/proof/src/run_dag.ts:534

• severity: HIGH
• flagged-by: proof-runtime-internals
• risk: A network timeout, rate-limit response, or SDK-internal error from Cursor.models.list() propagates as an unhandled rejection, terminating the process and discarding the DAG entirely — no partial results, no resumable state, no user-readable error.
• minimal fix: Wrap in try/catch; on error, log a warning and fall back to unresolvedModelForComplexity (skipping catalog validation), or at minimum surface a clear error message before exiting rather than a raw stack trace.

packages/proof/src/canvas_writer.ts:236–244

• severity: MED
• flagged-by: proof-runtime-internals
• risk: ModelParameterValue and ModelSelection are duplicated inside the embedded canvas template string. TypeScript does not enforce their parity with the exported types in dag.ts. A field added to ModelSelection (e.g., a displayName hint for the canvas) will silently be absent from canvas rendering.
• minimal fix: Add a codegen or satisfies-style sanity check, or at minimum expand the // Keep in sync comment to list the exact fields that need mirroring so a reviewer can spot drift at a glance.

---

Coverage plan

1. packages/proof/src/dag.test.ts — positive — createCatalogBackedModelResolver cache hit: call resolver('HIGH') twice with the same catalog; assert resolveModelSelectionFromCatalog is only invoked once (verify via a spy or by injecting a catalog that would throw on second lookup).
2. packages/proof/src/dag.test.ts — negative — createCatalogBackedModelResolver propagates catalog errors: pass a catalog that throws for one complexity; assert the resolver surfaces the error on first call and does not cache a bad state.
3. packages/proof/src/dag.test.ts — edge — Cursor.models.list() transient failure: mock the SDK call to reject; assert the run exits with a readable message rather than an uncaught rejection.

---

Reviewer scoreboard

• proof-runtime-internals: 2 findings, 2 coverage gaps, signal: HIGH
• test-coverage-robustness: 0 findings, 1 coverage gap, signal: MED
• correctness-and-contracts: 0 new findings (ModelMap shape change intentional and documented; Proof unreleased), signal: LOW

  

_{Sent by Cursor Automation: Flatbread PR Review}